fix(css): parse scroll-state and general-enclosed queries#9284
Merged
denbezrukov merged 2 commits intomainfrom Mar 2, 2026
Merged
fix(css): parse scroll-state and general-enclosed queries#9284denbezrukov merged 2 commits intomainfrom
denbezrukov merged 2 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 8f24677 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
A-Parser
Contributor
WalkthroughThis PR fixes false-positive diagnostics for valid Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_css_parser/src/syntax/value/function.rs`:
- Around line 432-435: The recovery currently used by
ComponentValueExpressionList::recover and the
parsed_element.or_recover_with_token_set call only stops at ) and ; and
therefore can over-consume past commas or binary-operator boundaries; update the
recovery token set passed to parsed_element.or_recover_with_token_set (and the
recovery logic in ComponentValueExpressionList::recover) to also include comma
and binary-operator tokens (e.g. T![,], T![+], T![-], T![*], T![/] or your
project's equivalent operator token symbols) so recovery will stop at argument
separators and operators instead of swallowing the rest of the expression.
Ensure you reference and modify the ParseRecoveryTokenSet construction used in
the shown call and any corresponding recover method to include these tokens.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
crates/biome_css_formatter/tests/specs/css/scss/at-rule/container-general-enclosed.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_formatter/tests/specs/css/scss/at-rule/supports-general-enclosed.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/at_rule/at_rule_container/at_rule_container_general_enclosed_error.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/at_rule/at_rule_container/at_rule_container_scroll_state_error.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/at_rule/at_rule_supports/at_rule_supports_general_enclosed_error.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/at-rule/container-general-enclosed.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/error/scss/at-rule/supports-general-enclosed.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_container_general_enclosed.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_container_scroll_state.css.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/container-general-enclosed.scss.snapis excluded by!**/*.snapand included by**crates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/container-scroll-state.scss.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (17)
.changeset/fix-false-positive-container-query-diagnostics.mdcrates/biome_css_formatter/tests/specs/css/scss/at-rule/container-general-enclosed.scsscrates/biome_css_formatter/tests/specs/css/scss/at-rule/supports-general-enclosed.scsscrates/biome_css_parser/src/syntax/at_rule/container/mod.rscrates/biome_css_parser/src/syntax/at_rule/supports/mod.rscrates/biome_css_parser/src/syntax/mod.rscrates/biome_css_parser/src/syntax/value/function.rscrates/biome_css_parser/src/syntax/value/url.rscrates/biome_css_parser/tests/css_test_suite/error/at_rule/at_rule_container/at_rule_container_general_enclosed_error.csscrates/biome_css_parser/tests/css_test_suite/error/at_rule/at_rule_container/at_rule_container_scroll_state_error.csscrates/biome_css_parser/tests/css_test_suite/error/at_rule/at_rule_supports/at_rule_supports_general_enclosed_error.csscrates/biome_css_parser/tests/css_test_suite/error/scss/at-rule/container-general-enclosed.scsscrates/biome_css_parser/tests/css_test_suite/error/scss/at-rule/supports-general-enclosed.scsscrates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_container_general_enclosed.csscrates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_container_scroll_state.csscrates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/container-general-enclosed.scsscrates/biome_css_parser/tests/css_test_suite/ok/scss/at-rule/container-scroll-state.scss
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/biome_css_parser/src/syntax/value/function.rs (2)
114-119: Hardcoded offset ofn + 3for SCSS qualified names.This assumes SCSS qualified names are always exactly 3 tokens. If the structure ever changes, this will silently break. Consider adding a brief comment clarifying the expected token structure (e.g.,
namespace.name).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/src/syntax/value/function.rs` around lines 114 - 119, The check using a hardcoded offset n + 3 when validating SCSS qualified names can break if token layout changes; update the code around the function that returns bool (the block that calls is_nth_at_identifier, context.is_scss_syntax_allowed(), and is_nth_at_scss_qualified_name) to add a short clarifying comment explaining the expected token layout for SCSS qualified names (e.g., "expects namespace . name => tokens: IDENT, '.', IDENT, '(' so we check n+3 for '('"), and keep the existing n + 3 check but document why that offset is used so future readers know the assumed structure; reference is_nth_at_scss_qualified_name and the p.nth_at(n + 3, T!['(']) call when adding the comment.
83-91: Consider adding rustdoc for consistency.
is_at_css_functionandis_at_function_with_contextlack documentation whilst similar functions likeis_at_function(line 74-77) have it. A brief doc comment would aid discoverability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_css_parser/src/syntax/value/function.rs` around lines 83 - 91, Add brief rustdoc comments for the two helper functions so their intent matches surrounding documented APIs: document is_at_css_function(p: &mut CssParser) to state it checks for a simple function head in CSS-only mode and document is_at_function_with_context(p: &mut CssParser, context: ValueParsingContext) to state it checks for a function head using the supplied ValueParsingContext (and mention that it excludes url() via its implementation). Keep the comments concise and consistent in style with the existing doc on is_at_function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_css_parser/src/syntax/value/function.rs`:
- Around line 114-119: The check using a hardcoded offset n + 3 when validating
SCSS qualified names can break if token layout changes; update the code around
the function that returns bool (the block that calls is_nth_at_identifier,
context.is_scss_syntax_allowed(), and is_nth_at_scss_qualified_name) to add a
short clarifying comment explaining the expected token layout for SCSS qualified
names (e.g., "expects namespace . name => tokens: IDENT, '.', IDENT, '(' so we
check n+3 for '('"), and keep the existing n + 3 check but document why that
offset is used so future readers know the assumed structure; reference
is_nth_at_scss_qualified_name and the p.nth_at(n + 3, T!['(']) call when adding
the comment.
- Around line 83-91: Add brief rustdoc comments for the two helper functions so
their intent matches surrounding documented APIs: document is_at_css_function(p:
&mut CssParser) to state it checks for a simple function head in CSS-only mode
and document is_at_function_with_context(p: &mut CssParser, context:
ValueParsingContext) to state it checks for a function head using the supplied
ValueParsingContext (and mention that it excludes url() via its implementation).
Keep the comments concise and consistent in style with the existing doc on
is_at_function.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
crates/biome_css_parser/tests/css_test_suite/error/function/component_value_expression_recovery.css.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (2)
crates/biome_css_parser/src/syntax/value/function.rscrates/biome_css_parser/tests/css_test_suite/error/function/component_value_expression_recovery.css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The root issue was that parsing fallback paths for unknown/general-enclosed syntax used
SCSS-aware value parsing even in .css files.
That caused false-positive diagnostics like “Expected a SCSS expression” for CSS
constructs.
This change splits value parsing into two modes:
I'd like to try to keep parse_exclusive_syntax because it's useful because it gives both:
Test Plan